Skip to content

Sphinx fixes #2681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 24 commits into from
Closed

Sphinx fixes #2681

wants to merge 24 commits into from

Conversation

emmanuelle
Copy link
Contributor

I'll be working on fixing the sphinx documentation, but this is a first PR to store the generated files in a sphinx artifact so that reviewers can check the result without checking out the branch. With this idea in mind, I did not zip the output so that the html doc can be browsed directly on circleCI.

@emmanuelle
Copy link
Contributor Author

emmanuelle commented Aug 3, 2020

in 75522ec I removed all the transformations between graph_objs and graph_objects because this seems to be the cause of the problem (I initially suspected the lazy import but apparently it's something else). Therefore we'll have the doc artifact as a reference to check that everything's ok with graph_objs. In the next commit I'll try to make it work with graph_objects

@emmanuelle
Copy link
Contributor Author

@nicolaskruchten still WIP but you can already take a look at the artifact to see if everything's ok (the problems mentioned in #2652 seem to be solved I think).
There were several problems:

  • probably one related to a lazy import problem. I switched to py 3.7 to build the doc.
  • one needs to pip install -e . plotly to be sure that the hacks done to the source files (graph_objs/graph_objects are taken into account). With this change the apidoc looks now correct to me, but in the doc_prod branch where plotly is the released version this won't work. I plan to uninstall plotly and install from the local version just before the build of the API doc, this should not be a problem. (in fact I'm not even sure why we need to use the released version of plotly when in doc-prod because the code should be up to date with the last release, just the doc can be different). This will be for tomorrow, it's already late here :-).

@nicolaskruchten
Copy link
Contributor

OK great! Thank you :)

I plan to uninstall plotly and install from the local version just before the build of the API doc, this should not be a problem

I think we used to do this, so that's not a problem for me.

Also note I merged doc-prod into master today which contains the new look'n'feel for the apidoc so if you rebase onto master you'll get the new look.

@emmanuelle emmanuelle changed the title [WIP] Sphinx artifact Sphinx fixes Aug 4, 2020
@emmanuelle
Copy link
Contributor Author

OK, I think this one is ready now. Is it worth a changelog item (as a bug fix? Not sure we want to explain all the graph_objs/graph_objects problems :-))

@nicolaskruchten
Copy link
Contributor

No need for a changelog item, nope! Should this target doc-prod instead of master so it gets out immediately?

💃 for a squash-and-merge rather than a normal merge please :)

@nicolaskruchten
Copy link
Contributor

We can probably close this one out now, thanks for merging the other one. This stuff will get back-merged into master naturally at some point soon.

@emmanuelle emmanuelle closed this Aug 5, 2020
@archmoj archmoj deleted the sphinx-artifact branch November 23, 2021 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants